Skip to content

Added buffer size for used BufferedOutputStream#16

Closed
kowalczm wants to merge 2 commits intobytefish:masterfrom
kowalczm:master
Closed

Added buffer size for used BufferedOutputStream#16
kowalczm wants to merge 2 commits intobytefish:masterfrom
kowalczm:master

Conversation

@kowalczm
Copy link
Copy Markdown

Upgraded postgrsql jdbc driver to 42.2.2

kowalczm and others added 2 commits March 23, 2018 10:28
@kowalczm kowalczm changed the title Upgraded postgresql jdbc Added buffer size for used BufferedOutputStream Mar 23, 2018
@bytefish
Copy link
Copy Markdown
Owner

Thanks for the Pull Request! 🙏 I will need some time to review it. Do you have any figures on performance improvements?

@kowalczm
Copy link
Copy Markdown
Author

kowalczm commented Mar 25, 2018

I have played with 'IntegrationTest.java' and see that having configrable buffer size we can control how many calls are done for Postgres Db and see significant performance improvment when sending data. So for larger buffer size than default in PGCopyOutputStream (65536) we are deacresing calls to db. Additionality byte array copying from BufferedOutputStream to PGCopyOutputStream is removed when BufferedOutputStream buffer size is less that PGCopyOutputStream buffer size.

@bytefish
Copy link
Copy Markdown
Owner

bytefish commented May 19, 2018

Sorry for my very late reply. I am now reviewing it.

bytefish added a commit that referenced this pull request May 19, 2018
@bytefish
Copy link
Copy Markdown
Owner

bytefish commented May 19, 2018

@kowalczm Could you explain, why you are setting the Buffer Size to 1 for the PGCopyOutputStream? Maybe I am not understanding Java good enough.

new DataOutputStream(new BufferedOutputStream(new PGCopyOutputStream(copyIn, 1), bufferSize));

bytefish added a commit that referenced this pull request May 19, 2018
@bytefish
Copy link
Copy Markdown
Owner

@kowalczm Ah I get it. The PgCopyOutputStream has some range checks and if the buffer size is 1, then it will always write as soon as the BufferedOutputStream flushes. Nice one. 👍

bytefish added a commit that referenced this pull request May 19, 2018
Improvements to PgBinaryWriter. Thanks a lot to Mariusz (@kowalczm)!
bytefish added a commit that referenced this pull request May 19, 2018
@bytefish
Copy link
Copy Markdown
Owner

Yes, I have read through the Postgres JDBC Driver code. I see two options here...

  1. Remove the BufferedOutputStream, because the PGCopyOutputStream is already buffered.
  2. Set the PGCopyOutputStream buffer size to 1 and use a BufferedOutputStream.

If one uses the 2. approach, then at the end of the day the PGCopyOutputStream simply dispatches writes through to the CopyIn. This will indeed stop any Memory Copying Operations introduced by me due to a small Buffer Size in the BufferedOutputStream.

I have opted to go for the 1. option, that you have used as well. If it mysteriously breaks, then we will use the 2. approach. The PgBulkInsert class now accepts a IConfiguration to adjust the Buffer Size.

bytefish added a commit that referenced this pull request May 19, 2018
Add a Configuration class to modify the Write Buffer Size for possible
Performance modifications. Add a Default Configuration. Use the
Configuration in the PgBulkInsert class.
bytefish added a commit that referenced this pull request May 19, 2018
@bytefish
Copy link
Copy Markdown
Owner

Released Version 2.1 with a Configurable Buffer Size. Thanks! 👍

@bytefish bytefish closed this May 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants